Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prefer JS over TS in extension resolve #9953

Closed
wants to merge 1 commit into from
Closed

Prefer JS over TS in extension resolve #9953

wants to merge 1 commit into from

Conversation

lifeiscontent
Copy link
Contributor

@lifeiscontent lifeiscontent commented Jan 5, 2020 •
edited
Loading

this fixes an issue when next.js is loading packages using typescript that compile their typescript code in-line with their .ts files.

e.g.

./
./a.ts => ./a.js
./b.ts => ./b.js

which would result in

./
./a.js
./a.ts
./b.js
./b.ts

@lifeiscontent
Update webpack-config.ts
3a8b967 
prefer JS over TS in resolve so that node_modules that compile .ts and .js in the same directory load the compiled result.
@ijjk
Copy link
Member

ijjk commented Jan 5, 2020

Stats from current PR

Default Server Mode
General
zeit/next.js canary lifeiscontent/next.js patch-1 Change
buildDuration 13.8s 13.7s -49ms
nodeModulesSize 48.9 MB 48.9 MB ✓
Client Bundles (main, webpack, commons)
zeit/next.js canary lifeiscontent/next.js patch-1 Change
main-HASH.js gzip 5.13 kB 5.13 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB ✓
commons.HASH.js gzip 4.06 kB 4.06 kB ✓
de003c3a9d30..4cf7.js gzip 13.7 kB 13.7 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 67.5 kB 67.5 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary lifeiscontent/next.js patch-1 Change
main-HASH.module.js gzip 4.19 kB 4.19 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB ✓
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 62.1 kB 62.1 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary lifeiscontent/next.js patch-1 Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB ✓
Overall change 4.76 kB 4.76 kB ✓
Client Pages
zeit/next.js canary lifeiscontent/next.js patch-1 Change
_app.js gzip 1.33 kB 1.33 kB ✓
_error.js gzip 4.07 kB 4.07 kB ✓
hooks.js gzip 779 B 779 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.9 kB 2.9 kB ✓
routerDirect.js gzip 283 B 283 B ✓
withRouter.js gzip 282 B 282 B ✓
Overall change 9.87 kB 9.87 kB ✓
Client Pages Modern
zeit/next.js canary lifeiscontent/next.js patch-1 Change
_app.module.js gzip 757 B 757 B ✓
_error.module.js gzip 3.06 kB 3.06 kB ✓
hooks.module.js gzip 371 B 371 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 2.47 kB 2.47 kB ✓
routerDirect..dule.js gzip 273 B 273 B ✓
withRouter.m..dule.js gzip 272 B 272 B ✓
Overall change 7.41 kB 7.41 kB ✓
Client Build Manifests
zeit/next.js canary lifeiscontent/next.js patch-1 Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Rendered Page Sizes
zeit/next.js canary lifeiscontent/next.js patch-1 Change
index.html gzip 1.02 kB 1.02 kB ✓
link.html gzip 1.03 kB 1.03 kB ✓
withRouter.html gzip 1.02 kB 1.02 kB ✓
Overall change 3.07 kB 3.07 kB ✓
Serverless Mode
General
zeit/next.js canary lifeiscontent/next.js patch-1 Change
buildDuration 14.3s 14s -291ms
nodeModulesSize 48.9 MB 48.9 MB ✓
Client Bundles (main, webpack, commons)
zeit/next.js canary lifeiscontent/next.js patch-1 Change
main-HASH.js gzip 5.13 kB 5.13 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB ✓
commons.HASH.js gzip 4.06 kB 4.06 kB ✓
de003c3a9d30..4cf7.js gzip 13.7 kB 13.7 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 67.5 kB 67.5 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary lifeiscontent/next.js patch-1 Change
main-HASH.module.js gzip 4.19 kB 4.19 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB ✓
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 62.1 kB 62.1 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary lifeiscontent/next.js patch-1 Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB ✓
Overall change 4.76 kB 4.76 kB ✓
Client Pages
zeit/next.js canary lifeiscontent/next.js patch-1 Change
_app.js gzip 1.33 kB 1.33 kB ✓
_error.js gzip 4.07 kB 4.07 kB ✓
hooks.js gzip 779 B 779 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.9 kB 2.9 kB ✓
routerDirect.js gzip 283 B 283 B ✓
withRouter.js gzip 282 B 282 B ✓
Overall change 9.87 kB 9.87 kB ✓
Client Pages Modern
zeit/next.js canary lifeiscontent/next.js patch-1 Change
_app.module.js gzip 757 B 757 B ✓
_error.module.js gzip 3.06 kB 3.06 kB ✓
hooks.module.js gzip 371 B 371 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 2.47 kB 2.47 kB ✓
routerDirect..dule.js gzip 273 B 273 B ✓
withRouter.m..dule.js gzip 272 B 272 B ✓
Overall change 7.41 kB 7.41 kB ✓
Client Build Manifests
zeit/next.js canary lifeiscontent/next.js patch-1 Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Serverless bundles
zeit/next.js canary lifeiscontent/next.js patch-1 Change
_error.js gzip 77.8 kB 77.8 kB ✓
hooks.html gzip 1.06 kB 1.06 kB ✓
index.js gzip 78 kB 78 kB ✓
link.js gzip 80.4 kB 80.4 kB ✓
routerDirect.js gzip 78.1 kB 78.1 kB ✓
withRouter.js gzip 78.1 kB 78.1 kB ✓
Overall change 393 kB 393 kB ✓

Commit: 3a8b967

@timneutkens
Copy link
Member

That sounds like a bug in the particular package (wrong publish). Also note that next does not compile node_modules, meaning you’re customizing webpack config to achieve that. Changing this will make resolving in typescript projects slower.

@lifeiscontent
Copy link
Contributor Author

lifeiscontent commented Jan 5, 2020 •
edited
Loading

@timneutkens every other framework (gatsby/create-react-app) resolves node_modules via JS not TS.

this might not be the right fix, but it definitely feels like a bug somewhere in the pipeline.

Within the typescript ecosystem its pretty common to compile your JS files along side your TS.

just because you haven't happened to use a node_module which does this doesn't make it not valid.

once its a node_module, (after compilation/published) it would seem pretty weird to consume TS. (which is currently happening)

Timer
Timer requested changes Jan 6, 2020
View reviewed changes
Copy link
Member

@Timer Timer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems breaking, however, I wouldn't mind changing this in the future (Next 10).

@lifeiscontent
Copy link
Contributor Author

@timneutkens also for the record, this wouldn't be an issue (for me) if styled-jsx worked in typescript alone, but since it doesn't I have to use babel to run the babel transforms to get the UI Kit I'm building to work correctly in different projects.

@Timer Timer added this to the backlog milestone Jan 20, 2020
@timneutkens
Copy link
Member

timneutkens commented Apr 6, 2020 •
edited
Loading

Did a bunch more investigation into this and it seems that the TS compiler prefers .js files too so let's mirror that behavior, could you open a new PR and add tests for this behavior in test/integration/typescript.

I'll have to close this PR as it's fork has disappeared and I can't do a checkout for it.

@timneutkens timneutkens closed this Apr 6, 2020
@Timer Timer mentioned this pull request Apr 19, 2020
Merged
@Timer Timer removed this from the backlog milestone Nov 16, 2020
This was referenced Sep 16, 2021
Open
Open
@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Timer Timer Timer requested changes

@ijjk ijjk Awaiting requested review from ijjk

@lfades lfades Awaiting requested review from lfades

@timneutkens timneutkens Awaiting requested review from timneutkens

Assignees
No one assigned
Labels
type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lifeiscontent @ijjk @timneutkens @Timer